feat: graduate --x-include-runtime
#8166
Conversation
🦋 Changeset detectedLatest commit: 5eef917 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-wrangler-8166You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/8166/npm-package-wrangler-8166Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-wrangler-8166 dev path/to/script.jsAdditional artifacts:cloudflare-workers-bindings-extension: wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-cloudflare-workers-bindings-extension-8166 -O ./cloudflare-workers-bindings-extension.0.0.0-ve39885045.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-ve39885045.vsixcreate-cloudflare: npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-create-cloudflare-8166 --no-auto-update@cloudflare/kv-asset-handler: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-cloudflare-kv-asset-handler-8166miniflare: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-miniflare-8166@cloudflare/pages-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-cloudflare-pages-shared-8166@cloudflare/unenv-preset: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-cloudflare-unenv-preset-8166@cloudflare/vite-plugin: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-cloudflare-vite-plugin-8166@cloudflare/vitest-pool-workers: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-cloudflare-vitest-pool-workers-8166@cloudflare/workers-editor-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-cloudflare-workers-editor-shared-8166@cloudflare/workers-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-cloudflare-workers-shared-8166@cloudflare/workflows-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13639768023/npm-package-cloudflare-workflows-shared-8166Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
emily-shen
force-pushed
the
emily/dynamic-typegen-ga
branch
from
56b09dc to
3d88fea
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
emily-shen
force-pushed
the
emily/dynamic-typegen-ga
branch
from
3d88fea to
68d6130
Compare
emily-shen
force-pushed
the
emily/dynamic-typegen-ga
branch
3 times, most recently
from
c86cbf3 to
88426fa
Compare
| @@ -0,0 +1,9 @@ | |||
| --- | |||
| "wrangler": minor | |||
There was a problem hiding this comment.
major? pretty sure there's nothing breaking. inclusion in v4 is mostly for politeness as there's a fair amount of CI usage
emily-shen
force-pushed
the
emily/dynamic-typegen-ga
branch
4 times, most recently
from
a7df791 to
c931fe0
Compare
emily-shen
force-pushed
the
emily/dynamic-typegen-ga
branch
from
9a873ab to
4872cb9
Compare
fixtures/type-generation/tests/type-generation.file-comment.test.ts
Outdated
Show resolved
Hide resolved
| try { | ||
| const { envHeader } = await generateEnvTypes( | ||
| config, | ||
| { strictVars: previousStrictVars === "false" ? false : true }, |
There was a problem hiding this comment.
| { strictVars: previousStrictVars === "false" ? false : true }, | |
| { strictVars: previousStrictVars !== "false" }, |
There was a problem hiding this comment.
I prefer the current one as its easier to understand (at least for me)
packages/wrangler/src/type-generation/runtime/log-runtime-types-message.ts
Outdated
Show resolved
Hide resolved
emily-shen
force-pushed
the
emily/dynamic-typegen-ga
branch
from
4872cb9 to
2ef9aa4
Compare
|
There's one thing I was thinking about: should we also update the C3 templates? Currently the Ideally the types should not be in a static file but generated, I have a pending PR that does that for Next. I think whatever If we decide to use the runtime types, /cc @petebacondarwin |
I'm actually working on a PR right now to do that :) It'll update C3 to run |
emily-shen
force-pushed
the
emily/dynamic-typegen-ga
branch
2 times, most recently
from
0e4ca2d to
a353f14
Compare
|
|
||
| Include runtime types in the output of `wrangler types` by default | ||
|
|
||
| `wrangler types` will now produce one file that contains both `Env` types and runtime types based on your compatibility date and flags. This is located at `worker-configuration.d.ts` by default. |
There was a problem hiding this comment.
Worth taking this opportunity to rename this file, maybe to cloudflare.d.ts? cc @irvinebroque if you have any thoughts here
| await ensureDirectoryExists(outFile); | ||
|
|
||
| const header = `// Runtime types generated with workerd@${version} ${compatibility_date} ${compatibility_flags.join(",")}`; | ||
| const header = `// Runtime types generated with workerd@${version} ${compatibility_date} ${compatibility_flags.sort().join(",")}`; |
There was a problem hiding this comment.
Since this is now a combined types file, the header should probably be Types generated with..., which would also bust all existing caches
There was a problem hiding this comment.
#8166 (comment)
also why do we want to bust existing caches?
|
Could we have a more unified single header? Something like: |
| { | ||
| "compilerOptions": { | ||
| ... | ||
| "types": ${updatedTypesString} |
There was a problem hiding this comment.
I don't think this is actually required? It seems to include that file for me by default
There was a problem hiding this comment.
huh yeah so it does. Is it because its covered by **/*.ts in include??
There was a problem hiding this comment.
Looks like the default is **/*: https://www.typescriptlang.org/tsconfig/#include
hmm we could, but since we can use cached runtime types independently of env changes, its slightly more convenient to just have that bit on its own line, and i don't think it'll particularly matter to users. happy to change it if you feel strongly about it. |
penalosa
left a comment
penalosa
left a comment
There was a problem hiding this comment.
Couple comments, but overall this is looking great!
| config, | ||
| envInterface, | ||
| outputPath | ||
| if ((header.length && content.length) || entrypointFormat === "modules") { |
There was a problem hiding this comment.
I realise this was probably here before, but I don't really understand this logic. Presumably at this stage there will always be something in header & content? What does the entrypointFormat check do?
There was a problem hiding this comment.
this is for the extremely niche situation where its a service syntax worker and --include-runtime=false - we don't want to write an empty Env interface for that case.
I have no idea why that would be a problem, but we do have a test making sure we don't... any ideas? 😅
|
|
||
| if (envOutOfDate || runtimeOutOfDate) { | ||
| logger.log( | ||
| "❓ It looks like your types might be out of date. Have you updated your config file since last running `wrangler types`?" |
There was a problem hiding this comment.
Could we make this more assertive? What's the recommended user action here?
Right now it's not clear what the user should do as a result of their answer to the question posed
| const isWorkersTypesInstalled = tsconfigTypes.find((type) => | ||
| type.startsWith("@cloudflare/workers-types") | ||
| ); | ||
| const isNodeTypesInstalled = tsconfigTypes.find((type) => type === "node"); |
There was a problem hiding this comment.
I'm not sure this logic is fully correct. If the types array is empty, but @types/node is in node_modules it'll automatically be picked up as well.
(I realise this is existing logic, but we might as well fix it up)
There was a problem hiding this comment.
600a895 fixed here, but i can't figure out how to test it because the tests always run in a context where @types/nodes exists 😢
emily-shen
force-pushed
the
emily/dynamic-typegen-ga
branch
from
939339f to
600a895
Compare
* graduate wrangler type generation to GA * fixups * extra changeset
* graduate wrangler type generation to GA * fixups * extra changeset
* graduate wrangler type generation to GA * fixups * extra changeset
* graduate wrangler type generation to GA * fixups * extra changeset
* graduate wrangler type generation to GA * fixups * extra changeset
* graduate wrangler type generation to GA * fixups * extra changeset
* ci: setup next branch with prereleases to next tag * chore: remove deprecated `getBindingsProxy` (#5005) * Require Node v18 in Wrangler v4 (#7338) * Remove node_compat in Wrangler & Miniflare v4 (#7336) * Remove v2 warning (#7228) * chore: update esbuild (#6884) * `--local` by default for `wrangler kv` & `wrangler r2` (#7392) * deprecate unused D1 commands/options (#7471) * chore: remove alpha support from `wrangler d1 migrations apply` * chore: remove `wrangler d1 backups` * chore: fixup tests * chore: fixup tests * chore: fixup tests * chore: remove --batch-size * Remove deprecated commands & config (#7352) * Remove wrangler version * Remove generate & strip down init * Add changesets * Remove deprecated --format & config * Add changeset * remove commands * Fix tests * Remove .only() * Remove legacy assets * clarify legacy assets -> sites naming * various test fixes after rebase * fixup! chore: update esbuild (#6884) esbuild 0.24.2 variable naming changes * fixup! Remove node_compat in Wrangler & Miniflare v4 (#7336) * feat(wrangler): Drop worker prefix when creating KV namespaces (#7759) * feat(wrangler): Drop worker prefix when creating KV namespaces --------- Co-authored-by: Samuel Macleod <[email protected]> * Include node version in name * Ignore kv-asset-handler tests on Node 22 * rename tests * Rename positional `json` args. Fixes #7688 * unescape * fix tests * Update .changeset/funny-pets-punch.md * Add system requirements to the README * fix formatting * fix formatting * Address feedback * lockfile * fix test * Fix changeset format * Use RC versioning * feat: graduate `--x-include-runtime` (#8166) * graduate wrangler type generation to GA * fixups * extra changeset * Fix legacy assets * Clarify that node support affects Wrangler & Miniflare & kv-asset-handler * Clarify comment re node 20 * Add more packages * Remove obsolete tests * Rename legacyAssets * more robust isLocal * fix test snapshot * Add --legacy-peer-deps so that installing Wrangler v4 doesn't case issues with frameworks that haven't updated their peer dependency for Wrangler v4 * Remove crypto polyfill * fix kv.local test after rebase * fix lockfile * bump rc version * revert rc versions --------- Co-authored-by: Dario Piotrowicz <[email protected]> Co-authored-by: Max Rozen <[email protected]> Co-authored-by: Pedro Leal <[email protected]> Co-authored-by: emily-shen <[email protected]>
* ci: setup next branch with prereleases to next tag * chore: remove deprecated `getBindingsProxy` (#5005) * Require Node v18 in Wrangler v4 (#7338) * Remove node_compat in Wrangler & Miniflare v4 (#7336) * Remove v2 warning (#7228) * chore: update esbuild (#6884) * `--local` by default for `wrangler kv` & `wrangler r2` (#7392) * deprecate unused D1 commands/options (#7471) * chore: remove alpha support from `wrangler d1 migrations apply` * chore: remove `wrangler d1 backups` * chore: fixup tests * chore: fixup tests * chore: fixup tests * chore: remove --batch-size * Remove deprecated commands & config (#7352) * Remove wrangler version * Remove generate & strip down init * Add changesets * Remove deprecated --format & config * Add changeset * remove commands * Fix tests * Remove .only() * Remove legacy assets * clarify legacy assets -> sites naming * various test fixes after rebase * fixup! chore: update esbuild (#6884) esbuild 0.24.2 variable naming changes * fixup! Remove node_compat in Wrangler & Miniflare v4 (#7336) * feat(wrangler): Drop worker prefix when creating KV namespaces (#7759) * feat(wrangler): Drop worker prefix when creating KV namespaces --------- Co-authored-by: Samuel Macleod <[email protected]> * Include node version in name * Ignore kv-asset-handler tests on Node 22 * rename tests * Rename positional `json` args. Fixes #7688 * unescape * fix tests * Update .changeset/funny-pets-punch.md * Add system requirements to the README * fix formatting * fix formatting * Address feedback * lockfile * fix test * Fix changeset format * Use RC versioning * feat: graduate `--x-include-runtime` (#8166) * graduate wrangler type generation to GA * fixups * extra changeset * Fix legacy assets * Clarify that node support affects Wrangler & Miniflare & kv-asset-handler * Clarify comment re node 20 * Add more packages * Remove obsolete tests * Rename legacyAssets * more robust isLocal * fix test snapshot * Add --legacy-peer-deps so that installing Wrangler v4 doesn't case issues with frameworks that haven't updated their peer dependency for Wrangler v4 * Remove crypto polyfill * fix kv.local test after rebase * fix lockfile * bump rc version * revert rc versions --------- Co-authored-by: Dario Piotrowicz <[email protected]> Co-authored-by: Max Rozen <[email protected]> Co-authored-by: Pedro Leal <[email protected]> Co-authored-by: emily-shen <[email protected]>
* ci: setup next branch with prereleases to next tag * chore: remove deprecated `getBindingsProxy` (#5005) * Require Node v18 in Wrangler v4 (#7338) * Remove node_compat in Wrangler & Miniflare v4 (#7336) * Remove v2 warning (#7228) * chore: update esbuild (#6884) * `--local` by default for `wrangler kv` & `wrangler r2` (#7392) * deprecate unused D1 commands/options (#7471) * chore: remove alpha support from `wrangler d1 migrations apply` * chore: remove `wrangler d1 backups` * chore: fixup tests * chore: fixup tests * chore: fixup tests * chore: remove --batch-size * Remove deprecated commands & config (#7352) * Remove wrangler version * Remove generate & strip down init * Add changesets * Remove deprecated --format & config * Add changeset * remove commands * Fix tests * Remove .only() * Remove legacy assets * clarify legacy assets -> sites naming * various test fixes after rebase * fixup! chore: update esbuild (#6884) esbuild 0.24.2 variable naming changes * fixup! Remove node_compat in Wrangler & Miniflare v4 (#7336) * feat(wrangler): Drop worker prefix when creating KV namespaces (#7759) * feat(wrangler): Drop worker prefix when creating KV namespaces --------- Co-authored-by: Samuel Macleod <[email protected]> * Include node version in name * Ignore kv-asset-handler tests on Node 22 * rename tests * Rename positional `json` args. Fixes #7688 * unescape * fix tests * Update .changeset/funny-pets-punch.md * Add system requirements to the README * fix formatting * fix formatting * Address feedback * lockfile * fix test * Fix changeset format * Use RC versioning * feat: graduate `--x-include-runtime` (#8166) * graduate wrangler type generation to GA * fixups * extra changeset * Fix legacy assets * Clarify that node support affects Wrangler & Miniflare & kv-asset-handler * Clarify comment re node 20 * Add more packages * Remove obsolete tests * Rename legacyAssets * more robust isLocal * fix test snapshot * Add --legacy-peer-deps so that installing Wrangler v4 doesn't case issues with frameworks that haven't updated their peer dependency for Wrangler v4 * Remove crypto polyfill * fix kv.local test after rebase * fix lockfile * bump rc version * revert rc versions --------- Co-authored-by: Dario Piotrowicz <[email protected]> Co-authored-by: Max Rozen <[email protected]> Co-authored-by: Pedro Leal <[email protected]> Co-authored-by: emily-shen <[email protected]>
4 participants
TODO: rebase on /vnext for inclusion in wrangler v4
Might be best to review commit by commit
This PR will turn on runtime type generation by default for
wrangler types.The
--experimental-include-runtimeflag will be superseded byinclude-runtimeandinclude-env, both of which aretrueby default.Also, the runtime types will be generated in the same file as the env types, at the location specified by path (
worker-configuration.d.tsby default).Before:
After:
